-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Deprecate BT_FIXED_APPKEY #25414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Deprecate BT_FIXED_APPKEY #25414
Conversation
Manifest update to depracate and replace BT_FIXED_PADDKEY Signed-off-by: alperen sener <[email protected]>
|
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
CI InformationTo view the history of this post, click the 'edited' button above Inputs:Sources:sdk-nrf: PR head: d30f08599c00858c5e1709d58db22f4575350576 more detailssdk-nrf:
zephyr:
Github labels
List of changed files detected by CI (20)Outputs:ToolchainVersion: df3cc9d822 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
|
You can find the documentation preview for this PR here. |
subsys/bluetooth/mesh/vnd/Kconfig
Outdated
| endif # BT_MESH_DM_SRV | ||
|
|
||
| config BT_MESH_LE_PAIR_RESP | ||
| bool "LE Pairing Responder model" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| bool "LE Pairing Responder model" | |
| bool "LE Pairing Responder model [EXPERIMENTAL]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 'Experimental'? Using this model allows fixed passkey to be not used by virtue of the architecture. Specification does not say that one should remove API or change APIs, the specification gives guideline that fixed passkey is not allowed. This model along with the underlaying API creates a system that make fulfilling this requirement possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because 2 lines down, you are selecting the EXPERIMENTAL Kconfig ?? https://github.com/nrfconnect/sdk-nrf/pull/25414/files#diff-6b6980f5c948cf835b4cfac2ecf56cb964fa0e31bd790325f3f17a351e23718dR90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, because it has experimental tag. Either we have to remove it or add what @nordicjm requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, if you select EXPERIMENTAL then you should clearly mark it as experimental in the title, that is the convention
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think smp_bt_auth.c for sure should not use the deprecated API and should only be compiled with CONFIG_BT_APP_PASSKEY.
But also I think that since the LE Pairing Responder model is experimental, it is OK to change it is behavior and API without deprecation period. But we need to mention this change in the release notes.
Also, remember to update documentation of the model: https://github.com/nrfconnect/sdk-nrf/blob/main/doc/nrf/libraries/bluetooth/mesh/vnd/le_pair_resp.rst.
2999999 to
0d6a557
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the documentation and added release notes for DFU distributer sample and LE Pair Responder model.
Remove all dependency on deprecated FIXED_PASSKEY, solely it depends on APP_PASSKEY now.
Removed immediate invalidation of passkey.
Update the Documentation diagram.
added commit to update matter door lock sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we show the usage of bt_mesh_le_pair_resp_passkey_get function on diagram? In the app_passkey callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes are required in the Matter Lock sample.
| uint32_t NUSService::AuthAppPasskey(bt_conn *conn) | ||
| { | ||
| return CONFIG_CHIP_NUS_FIXED_PASSKEY; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| uint32_t NUSService::AuthAppPasskey(bt_conn *conn) | |
| { | |
| return CONFIG_CHIP_NUS_FIXED_PASSKEY; | |
| } | |
| #if defined(CONFIG_BT_APP_PASSKEY) | |
| uint32_t NUSService::AuthAppPasskey(bt_conn *conn) | |
| { | |
| return CONFIG_CHIP_NUS_FIXED_PASSKEY; | |
| } | |
| #endif /* CONFIG_BT_APP_PASSKEY */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You need to declare this function in the
bt_nus_service.hfile: https://github.com/nrfconnect/sdk-nrf/blob/main/samples/matter/common/src/bt_nus/bt_nus_service.h#L88
as:
static uint32_t AuthAppPasskey(bt_conn *conn);
| @@ -31,6 +31,7 @@ BT_CONN_CB_DEFINE(conn_callbacks) = { | |||
| bt_conn_auth_cb Nrf::NUSService::sConnAuthCallbacks = { | |||
| .passkey_display = AuthPasskeyDisplay, | |||
| .cancel = AuthCancel, | |||
| .app_passkey = AuthAppPasskey, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .app_passkey = AuthAppPasskey, | |
| #if defined(CONFIG_BT_APP_PASSKEY) | |
| .app_passkey = AuthAppPasskey, | |
| #endif /* CONFIG_BT_APP_PASSKEY */ |
| @@ -381,6 +390,8 @@ Bluetooth Mesh samples | |||
| * Updated the :makevar:`FILE_SUFFIX` make variable to use more descriptive suffixes for external flash configurations. | |||
| The new suffixes are ``_dfu_ext_flash`` for external flash DFU storage and ``_ext_flash_settings`` for external flash settings storage. | |||
|
|
|||
| * Updated the sample to use the :kconfig:option:`CONFIG_BT_APP_PASSKEY` option instead of the deprecated :kconfig:option:`CONFIG_BT_FIXED_PASSKEY` option. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add the same release note entry to the Matter sample below this line: https://github.com/nrfconnect/sdk-nrf/blob/main/doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst?plain=1#L568
* Updated to use the :kconfig:option:`CONFIG_BT_APP_PASSKEY` option instead of the deprecated :kconfig:option:`CONFIG_BT_FIXED_PASSKEY` option.
BT_FIXED_PASSKEY is depracated so wee need to aling the le pair responder model with the new Kconfig BT_APP_PASSKEY usage. Adding bt_mesh_le_pair_resp_passkey_get to be able to retreive the passkey set randomly or by app. Signed-off-by: alperen sener <[email protected]>
BT_FIXED_PASSKEY is deprecated, thus we start using BT_APP_PASSKEY Signed-off-by: alperen sener <[email protected]>
BT_FIXED_APPKEY is deprecated. Signed-off-by: alperen sener <[email protected]>
0d6a557 to
d30f085
Compare
Deprecating BT_FIXED_PASSKEY and start using BT_APP_PASSKEY instead.